Skip to content

MPT-22436 Use freshly created product for e2e media tests#352

Open
jentyk wants to merge 1 commit into
mainfrom
claude/stupefied-morse-be6c21
Open

MPT-22436 Use freshly created product for e2e media tests#352
jentyk wants to merge 1 commit into
mainfrom
claude/stupefied-morse-be6c21

Conversation

@jentyk

@jentyk jentyk commented Jun 19, 2026

Copy link
Copy Markdown
Member

🤖 AI-generated PR — Please review carefully.

What

Fixes the e2e media tests (sync and async) that failed at setup with 400 Bad Request — Cannot create additional media as the maximum number of media items has been reached.

The media service fixtures built their service against the hardcoded product id from e2e_config (catalog.product.id, e.g. PRD-7255-3950). That product had reached its maximum number of media items, so every MediaService.create returned 400 and broke the media tests during fixture setup.

Changes

Point the media service fixtures at the shared created_product / async_created_product fixtures (already present in the product-level conftest.py) instead of the hardcoded product_id, so each run starts from a freshly created, empty product:

  • sync: vendor_media_servicecreated_product.id
  • async: async_media_service and async_vendor_media_serviceasync_created_product.id (pytest caches the fixture per test, so both share the same fresh product)

The created_product fixtures keep uploading the icon (file=logo_fd) — product creation requires it (see review thread), so the shared create_fixture_resource_and_delete helper can't be used here as-is.

Testing

  • Ran the related e2e suite against the live API (tests/e2e/catalog/product/media + tests/e2e/catalog/product product tests): 28 passed.
  • ruff check, ruff format --check, and flake8 (incl. wemake / flake8-aaa) pass on the changed files.

Jira: https://softwareone.atlassian.net/browse/MPT-22436

🤖 Generated with Claude Code

Closes MPT-22436

Changes

  • Fixed failing e2e media tests (sync and async) that were hitting maximum media capacity on a hardcoded product ID
  • Moved created_product and async_created_product fixtures to shared conftest.py to enable reuse across test modules
  • Refactored product fixtures to use shared create_fixture_resource_and_delete and async_create_fixture_resource_and_delete helpers
  • Updated media service fixtures to use freshly created products (created_product.id and async_created_product.id) instead of hardcoded product ID from configuration
  • Removed unused MPTAPIError imports from product test files
  • Each test now runs with an empty, isolated product that is cleaned up after execution

@jentyk jentyk requested a review from a team as a code owner June 19, 2026 09:31
@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown

✅ Found Jira issue key in the title: MPT-22436

Generated by 🚫 dangerJS against dadcf25

@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 8255b746-07f1-41ca-837b-6313ca390c7b

📥 Commits

Reviewing files that changed from the base of the PR and between b125076 and dadcf25.

📒 Files selected for processing (2)
  • tests/e2e/catalog/product/media/test_async_media.py
  • tests/e2e/catalog/product/media/test_sync_media.py
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • softwareone-platform/mpt-extension-skills (manual)
🚧 Files skipped from review as they are similar to previous changes (2)
  • tests/e2e/catalog/product/media/test_sync_media.py
  • tests/e2e/catalog/product/media/test_async_media.py
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: danger
  • GitHub Check: build

📝 Walkthrough

Walkthrough

Two test fixture signatures are updated in the sync and async media test files. The vendor_media_service, async_media_service, and async_vendor_media_service fixtures now accept the created_product/async_created_product fixture object and derive the product ID via .id instead of accepting a raw product_id parameter.

Changes

Media fixture rewiring

Layer / File(s) Summary
Media fixtures: accept product object instead of product_id
tests/e2e/catalog/product/media/test_sync_media.py, tests/e2e/catalog/product/media/test_async_media.py
vendor_media_service, async_media_service, and async_vendor_media_service fixtures replace the product_id parameter with the created_product/async_created_product fixture and scope the media client via .id.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Documentation Up To Date ✅ Passed PR changes are test-only refactoring of e2e fixture infrastructure (moving fixtures, updating imports) with no changes to source code, APIs, deployment, configuration, or user-facing behavior—docum...

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@jentyk jentyk force-pushed the claude/stupefied-morse-be6c21 branch from 87ba139 to 7bf4306 Compare June 19, 2026 09:36
Comment on lines +11 to +33
@pytest.fixture
def created_product(mpt_vendor, product_data, logo_fd):
product = mpt_vendor.catalog.products.create(product_data, file=logo_fd)

yield product

try:
mpt_vendor.catalog.products.delete(product.id)
except MPTAPIError as error:
print(f"TEARDOWN - Unable to delete product {product.id}: {error.title}") # noqa: WPS421


@pytest.fixture
async def async_created_product(async_mpt_vendor, product_data, logo_fd):
product = await async_mpt_vendor.catalog.products.create(product_data, file=logo_fd)

yield product

try:
await async_mpt_vendor.catalog.products.delete(product.id)
except MPTAPIError as error:
print(f"TEARDOWN - Unable to delete product {product.id}: {error.title}") # noqa: WPS421

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check the helpers: async_create_fixture_resource_and_delete and create_fixture_resource_and_delete

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — both created_product and async_created_product now use the shared create_fixture_resource_and_delete / async_create_fixture_resource_and_delete context-manager helpers from tests/e2e/helper.py, which removes the hand-rolled create + try/except-delete teardown.

One side effect: those helpers call service.create(resource_data) without a file=, so the product is no longer created with the logo_fd icon. The public OpenAPI spec marks neither product nor icon as required on POST /public/v1/catalog/products, so creating without an icon is valid — and these fixtures only need a product to exist so media/etc. can be attached.

🤖 Generated by AI

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update after running the e2e suite — I had to back this out. Creating a product through the helper drops the icon, and the live API rejects that:

400 One or more validation errors occurred.
{ "Icon": ["A value for the 'Icon' parameter or property was not provided."] }

(The OpenAPI spec doesn't mark Icon as required, which is what misled me earlier.) So created_product / async_created_product stay on the existing hand-rolled fixtures that upload logo_fd — the same ones #351 just merged — and this PR now only repoints the media service fixtures at them. All 28 product + media e2e tests pass this way.

Using the shared helper here would mean teaching it to forward a file= kwarg — happy to do that in a follow-up if you'd prefer.

🤖 Generated by AI

@jentyk jentyk force-pushed the claude/stupefied-morse-be6c21 branch from 7bf4306 to a1e2896 Compare June 19, 2026 09:54
@jentyk jentyk requested a review from albertsola June 19, 2026 09:56
@jentyk

jentyk commented Jun 19, 2026

Copy link
Copy Markdown
Member Author

@albertsola ready for re-review 🙏

Both created_product / async_created_product now use the shared create_fixture_resource_and_delete / async_create_fixture_resource_and_delete helpers, as you suggested. The only behavioural change is that the product is no longer created with an icon — the OpenAPI spec marks neither product nor icon as required on POST /catalog/products, and the fixtures just need a product to exist. Details in the thread reply.

🤖 Generated by AI

@jentyk jentyk force-pushed the claude/stupefied-morse-be6c21 branch from a1e2896 to b125076 Compare June 19, 2026 10:15
…-22436]

The media service fixtures built their service against the hardcoded
product id from e2e_config (catalog.product.id), which had reached its
maximum number of media items. MediaService.create then returned 400
Bad Request, failing setup for the sync and async media tests.

Point the sync and async media service fixtures at the shared
created_product / async_created_product fixtures so each run starts from
a freshly created, empty product instead of the shared seeded one.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jentyk jentyk force-pushed the claude/stupefied-morse-be6c21 branch from b125076 to dadcf25 Compare June 19, 2026 10:46
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants